Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: useSSE() basic implement #60

Closed
wants to merge 1 commit into from

Conversation

MeetinaXD
Copy link
Contributor

No description provided.

const fullURL = buildCompletedURL(baseURL, url, params);
const eventSource = new EventSource(fullURL, { withCredentials: config.withCredentials });

const { data, update } = useRequest(handler, config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里为什么要通过useRequest来创建data,是不是直接$(config.initialData)就可以了

Comment on lines +57 to +78
const createSSEEvent = (eventName: string, event: MessageEvent<any> | Event) => {
if (eventName === 'open') {
return {
method: methodInstance,
eventSource
} as AlovaSSEEvent;
}
if (eventName === 'error') {
return {
method: methodInstance,
eventSource,
error: new Error('sse error')
} as AlovaSSEErrorEvent;
}

// 其余名称的事件都是(类)message 的事件,data 交给 dataHandler 处理
return {
method: methodInstance,
eventSource,
data: dataHandler((event as MessageEvent).data)
} as AlovaSSEMessageEvent<any>;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可能你没注意到createHookEvent这个文件,应该是可以使用这个函数,然后调整下对SSE的事件对象的支持能用上。


return {
readyState: _exp$(readyState),
data,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个最好也可以_exp$一下,在react中可直接用

Comment on lines +358 to +360
if (callbacks) {
return forEach(callbacks, fn => fn(args));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个判断好像无效,因为按你写的callbacks至少都会是一个空数组

const setCallback = (fn: Fn) => {
if (!callbacks.includes(fn)) {
callbacks.push(fn);
onCallbackChange && onCallbackChange(callbacks);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我提下我的见解,如果我的话,我会把onCallbackChange接收处默认为noop,这样就不需要多个地方判断onCallbackChange是否有值,当然你这样写也没啥问题。


// type: eventname & useCallback()
const eventMap: Map<string, ReturnType<typeof useCallback>> = new Map();
const [onOpen, triggerOnOpen, offOpen] = useCallback<SSEOnOpenTrigger>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我觉得这个useCallback非常棒

@MeetinaXD
Copy link
Contributor Author

关闭这个 PR,最新的提交已经转移到这里:#62

Close this PR, the latest submission has been transferred here: #62

@MeetinaXD MeetinaXD closed this Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants